Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add nix language parser #4020

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

bsima
Copy link

@bsima bsima commented Jun 14, 2024

This is a work in progress. Okay I think this is in good shape now, seems to work well in my usage and passes unit tests.

optlib/nix.ctags Outdated Show resolved Hide resolved
source.mak Outdated Show resolved Hide resolved
@masatake
Copy link
Member

To docs/news/HEAD.rst, please add:

* Nix *optlib* by YOURNAME 

@masatake
Copy link
Member

Thank you.

@bsima bsima force-pushed the nix-ctags branch 2 times, most recently from 4745a10 to 7258205 Compare June 19, 2024 01:37
@bsima
Copy link
Author

bsima commented Jun 19, 2024

I updated the commit with your comments. Next I'll work on expanding the test case to actually test real-world nix code: I'll make a small library of functions and a couple builders with different formatting.

Copy link
Member

@masatake masatake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may have to put "NixParser" here:

MyrddinParser, \
.

After doing "make", you can run the test cases with "make units LANGUAGES=Nix".

@bsima bsima force-pushed the nix-ctags branch 2 times, most recently from 2838f80 to 6e0ea01 Compare June 20, 2024 21:47
@bsima
Copy link
Author

bsima commented Jul 11, 2024

I added some better test nix code, I tried to look at some of my personal nix code and copy a few formatting patterns that I use, which is similar to the style you see in upstream nixpkgs afaik. So I would expect this optlib to work reasonably well everywhere.

I thought I would need a multiline regex for the multilineFunc and multiline_attrset but I don't? It just worked? Maybe I don't understand regex as well as I thought?

Anyway sorry for taking so long to finish this. But I've been using this optlib in my personal code, which runs over nixpkgs and indeed finds most definitions I need, and my work code, which has thousands of lines of custom nix and it seems to work more often than not.

@bsima bsima marked this pull request as ready for review July 11, 2024 01:43
Copy link

codecov bot commented Jul 11, 2024

Codecov Report

Attention: Patch coverage is 0% with 21 lines in your changes missing coverage. Please review.

Project coverage is 85.39%. Comparing base (08e07dc) to head (8dfde48).
Report is 37 commits behind head on master.

Files Patch % Lines
optlib/nix.c 0.00% 21 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4020      +/-   ##
==========================================
- Coverage   85.43%   85.39%   -0.04%     
==========================================
  Files         235      236       +1     
  Lines       56727    56750      +23     
==========================================
  Hits        48462    48462              
- Misses       8265     8288      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@masatake
Copy link
Member

Could you rebase your chnages on the latest code?

--map-Nix=+.nix

# Index packages when we see "name = <tag>" or "pname = <tag>"
--kinddef-Nix=p,package,package definition
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as reading the test case, package is not tested at all.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doh, good catch, i fixed the regex here

optlib/nix.ctags Outdated
--kinddef-Nix=f,function,function definition
--regex-Nix=/(\S+)\s*=\s+\w+:/\1/f/

# Attrs definitions just have =, but only index if they have >=4 chars,
Copy link
Member

@masatake masatake Jul 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for rejecting short names looks weak.

One of the mottos of Universal Ctags design is to "prepare raw information to ctags client tools like vim when ctags doesn't know what's incorrect."

Rejecting can be done in a client tool reading the tags file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, removed that part

@masatake
Copy link
Member

The multiline regex meta parser is a toy in many situations.
The multi-table byte-oriented meta parser (--_tabledef-, and --_mtable-regex-) is the way to go.

I don't know the Nix language at all. However, I guess you may want to fill out the scope fields for tag entries. Without it, a client tool like Vim cannot draw a tree, which is useful for navigation.

struct point {
   int x;
   int y;
};

For the input of C language, ctags can emit the following tags:

point	input.c	/^struct point {$/;"	s	file:
x	input.c	/^  int x;$/;"	m	struct:point	typeref:typename:int	file:
y	input.c	/^  int y;$/;"	m	struct:point	typeref:typename:int	file:

From the tags output, x is a part of point.

  attrset = {
    foo = "bar";
  };

The current nix parser doesn't extract foo because the name is too short.
I guess an ideal Nix parser may emit:

attrest	input.nix	/^  attrset = {$/;"	a
foo	input.nix		;"	a	attr:attrest

Too many tags may not be a problem if the scope fields are filled out well.

However, these improvements can be made after merging this pull request.

@bsima
Copy link
Author

bsima commented Jul 17, 2024

Ah, scoped tags is a very good idea. It's something I would need if I want to
continue using tags in nixpkgs. Currently I just scan through the list of
duplicated tags for the correct one, but scopes is a much better solution. I
will read about scopes tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants